fix(plugin-reports)!: a non-member schedule timezone no longer discards the cron expression - #16878

Fixes #16291

Changeset level corrected after review feedback. It was patch; it is now minor and declared breaking, with an ADR-0087 disposition. The axis that decides is not the accept-set one — that turned out to be measurably dead — but the withdrawal of a currently-running delivery. Full re-derivation, with both measurements, under "Changeset level" below.

The three-state probe, re-run on this container

Triage asked for the croner behaviour to be re-measured rather than inherited. croner 10.0.1, Node v22.22.2:

new Cron('0 9 * * *', { timezone: 'Mars/Olympus' })                  -> constructs FINE  (returns an R)
  .nextRun(2026-09-08T00:00:00Z)                                     -> TypeError: CronDate: Failed to convert date
                                                                        to timezone 'Mars/Olympus' ... Invalid time
                                                                        zone specified: Mars/Olympus
new Cron('0 9 * * *', { timezone: 'Mars/Olympus' }, async () => {})  -> THROWS at construction (same TypeError)

Confirmed exactly as the card states. That three-state answer is the whole mechanical basis of this change: the eager guard used the callback-less form, so it validated the expression and was blind to the zone, and nextRunAt caught the deferred throw and returned from + interval_minutes. A report authored as "every weekday 09:00 Asia/Shanghai" fired every 1440 minutes, forever, re-derived on every sweep via advanceSchedule / rowFromSchedule — verbatim the outcome the guard's own comment says it exists to prevent.

What changed

1. The create-time guard asks the right question, with the repo's one predicate.

scheduleReport now consults isValueDomainMember('iana_time_zone', …) from @objectstack/spec/shared — the same probe sys_report_schedule.timezone's valueDomain write gate uses since #15872. No second time-zone judgement was written in plugin-reports; the predicate is imported. The refusal is VALIDATION_FAILED: invalid timezone 'ZONE': not a member of the 'iana_time_zone' value domain, which names the input that is actually wrong.

It applies whether or not a cron_expression is set, because the storage gate does too. A guard that accepted a value here for interval schedules would hand the engine a row it is about to refuse, and the divergence would surface as a generic field error — the exact regression #16291 records against #15872's door.

The row now also stores the string the scheduler evaluates: timezone: '' used to be stored verbatim while every new Cron call site read it as UTC via || 'UTC', so the stored value was one the storage gate refuses and the scheduler silently treated as something else. It is normalised to UTC on the way in.

2. Both warn paths, not one.

:821 and :823 each named only the cron expression. Both now name the expression and the zone, and the second no longer asserts the expression is the broken half:

A deliberate note on shape: an explicit if (!isUsableTimezone(tz)) branch inside nextRunAt was written first and then removed. With the sweep quarantine below in place, nothing can reach nextRunAt with a non-member zone — that branch was unreachable, i.e. a phantom check no test could ever drive red. The precise time-zone diagnosis lives where it is reachable (dispatchDue); nextRunAt names both inputs and accuses neither, which is what makes both of its paths honest on either cause.

3. Existing rows — the explicit answer triage asked for: MARK AND STOP, not repair.

#15872 refuses new bad rows at the column, and this PR refuses them at the service door, but neither heals a row already stored: valueDomain is in the written-values-only transition-gate class, so pre-existing rows are never re-validated. On a sweep, a schedule that has a cron_expression and whose stored zone is not an iana_time_zone member is now not run, its next_run_at is not advanced, and last_status: 'failed' / last_error carry the zone, the expression and what to do about it.

Why not the one-time repair path: the intended zone is not recoverable from a typo, and rewriting it to UTC would deliver at yet another set of wrong instants while the row looks healthy — trading a visible fault for an invisible one.

Three sub-decisions inside that, each deliberate:

• active is left set and next_run_at is left in the past. That is the same posture this loop already takes for a schedule whose report has vanished, and it means correcting the zone resumes the schedule on the very next sweep — no re-enable, no second admin action. (Covered by a test.)
• Guarded on cron_expression. Interval arithmetic never reads the zone, so an interval-only schedule carrying a legacy bad value still delivers on the cadence its author asked for and is left completely alone. (Covered by a test.)
• failed++ / last_status: 'failed', matching this loop's existing fail-closed arm for an unresolvable owner context, rather than a new status value — see the scope note below.

4. nextRunAt stays pure. It computes an instant; it does not decide policy and cannot write a row. The mark-and-stop decision lives in dispatchDue, where the row and the engine are in hand.

Changeset level — re-derived, with the measurement that overturned my first answer

The changeset was graded patch. It is now minor, declared breaking, with an ADR-0087 disposition. Two candidate axes were put to it; they did not both survive.

Axis 1 — "it refuses input it used to accept" — DOES NOT HOLD. Measured.

This looked like the obvious one, and it is wrong. scheduleReport refusing a non-member zone is only a real narrowing if a non-member zone could reach storage today. It cannot. Booted on a real ObjectKernel with the real ObjectQLPlugin and a real SqlDriver over SQLite, inserting the row directly:

insert sys_report_schedule { timezone: 'Mars/Olympus', cron_expression: '0 9 * * 1-5' }
  -> REFUSED: VALIDATION_FAILED
     Timezone must be a valid IANA time zone identifier, e.g. Europe/Zurich (got "Mars/Olympus")

insert sys_report_schedule { timezone: 'Asia/Shanghai',  cron_expression: '0 9 * * 1-5' }
  -> INSERTED
stored rows: ["rsch_good=Asia/Shanghai"]

So on origin/main today, scheduleReport with a bad zone already throws, and already throws VALIDATION_FAILED. This PR does not narrow a reachable accept set: it moves the refusal to an earlier door and gives it a message that names the timezone instead of a generic field error — which is what the card asked for, and is not a breaking act. The one honest residual is error shape, not accept set: the engine's error carries code: 'VALIDATION_FAILED' while the guard throws a bare Error whose message begins VALIDATION_FAILED: — the same spelling the other three guards in scheduleReport already use, so it is this door's existing convention rather than a new divergence.

Axis 2 — "schedules that run today stop running" — DOES hold, and it decides.

A row stored before valueDomain landed, carrying a cron expression and a non-member zone, is delivering right now on the interval cadence. After this change it delivers nothing until a human corrects the row. The upgrade requires operator action on stored data to restore a delivery that is happening today, and nothing restores it automatically.

That is the axis I am calling decisive, and I want to be explicit that it is not the "was the old behaviour ever promised?" axis. It was not promised — the object's own doc says a cron expression "is evaluated in timezone", so the interval fall-back was already a contract violation, and a pure correctness reading lands on patch. I did not take that reading, for one reason: the remedy is not automatic. Compare an RLS fix that stops returning rows a caller should never have seen — correct behaviour resumes by itself. Here it does not; a human must edit a row, and until they do a report an operator relies on is silently absent. During the launch window the level carries no breaking-ness at all (check-changeset-no-major refuses major, and the repo's own text says the BREAKING banner plus the ADR-0087 disposition are "the only signal there is"). A patch with neither would tell that operator there is nothing to look at.

What would falsify this call: if the quarantine were self-healing without human action (a fall back to UTC, or a repair migration), or if no stored row could be in this state. Neither holds — #15872's own comment states the column is written-values-only and that "no migration is owed", so pre-existing non-members survive by design. The set cannot grow (axis 1's probe proves the write door is shut), but a set that cannot grow is not an empty one.

The gates, and what they did and did not say

node scripts/check-adr-0087-registration.mjs --base origin/main
  -> exit 0
  ✓ 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.
      .changeset/report-schedule-timezone-guard.md  [BREAKING+bang]  not-required (no-migration-prescription)

node scripts/check-changeset-no-major.mjs --base origin/main
  -> exit 0 · ✓ no `major` bump
  ℹ️ LEVEL AXIS: NOT MEASURED — no clause-② declaration was readable offline

Because that last line is a non-reading rather than a pass, the level axis was then driven offline with --event, both ways, so the grade does not depend on which way clause ② is read:

--event (Clause-②: no)   -> ✓ LEVEL AXIS: declares clause-② `no` … (exit 0)
--event (Clause-②: yes)  -> ✓ LEVEL AXIS: declares clause-② `yes`, and no package … is graded `patch` (exit 0)

⚠️ Stated plainly so review is not misled: no gate forced this correction. Run against the previous head, which carried the patch grade:

check-adr-0087-registration --head df9d4c9b9
  -> ✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)
check-changeset-no-major --head df9d4c9b9 --event (Clause-②: yes)
  -> ✓ LEVEL AXIS … (exit 0)

Both were green on patch. The ADR-0087 gate is driven off the author's own declaration by design — its header says silence is the single thing it forbids, and a changeset declaring nothing has nothing demanded of it. And the level axis could not see this package at all: PUBLISHED_SOURCE_PATH = /^packages\/([^/]+)\/src\// matches one segment, so packages/plugins/plugin-reports/src/** is invisible to it (measured here: 47 of 70 publishable workspace packages are nested deeper than one segment). That blind spot is already filed as #16713 and is not re-filed here; this PR is a corroborating data point for it, nothing more.

So the correction rests on the argument above, not on a red gate — which is the right way round, and is why it is written out rather than asserted.

last_status / last_error — checked first, as directed

Both columns already exist on sys_report_schedule (packages/platform-objects/src/audit/sys-report-schedule.object.ts, group State), and ReportSchedule / rowFromSchedule already carry them. markSchedule already writes them on two other arms. So marking stays in this card and moves no schema.

What was deliberately kept out to avoid moving schema: last_status is Field.select(['ok', 'failed', 'skipped']). A more precise value such as 'misconfigured' would be a new enum member — that is a schema move, and per triage's instruction it is not taken here. 'failed' is used instead, with the precision carried in last_error.

Also out of scope, and not filed: this arm re-marks the row on every sweep, because the row stays due. That write amplification is pre-existing and identical in kind to the missing-report arm three lines below it, so making only the new arm idempotent would leave the function inconsistent. Noted for a maintainer, not carded.

Adjacent cards — left alone, as directed

#15872 (landed, the column's write door) and #16292 (CronScheduleSchema.timezone is a bare z.string(), domain:spec) are not closed or folded in here. Three doors, one predicate, three cards.

One sibling-package edit, and why

packages/platform-objects/src/audit/sys-report-schedule.object.ts carries a present-tense paragraph stating that scheduleReport's guard "does not catch it … and so is blind to exactly this half of its own input". This PR is what makes that false, so the sentence is moved to past tense and a [#16291] note records how each half is now closed. Comment-only; that package's full suite (545 tests) and typecheck were run.

Verification

The gate union, pnpm lint and both suites were re-run in full on deaa7b717, the final commit. The ablation legs were run on df9d4c9b9; report-service.ts is byte-identical at both (blob d11eff8969637d2e5737feea308adfdec8411d74), the only difference between the two heads being the changeset file.

• Gate union — derived from a fetched tree with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands; the union is the same 59 families before and after the changeset edit; all 59 run; reconciled with --ran: ✓ 59 derived famil(ies) accounted for — 59 run, 0 NOT-MEASURED. Every one exits 0.
• Two gates first answered PREREQUISITE NOT MET (exit 3) and were NOT reported as passes. check:dual-build-cjs-loads and check:i18n both named a missing build; a full pnpm build (73/73 tasks) was run and both were re-run to a real verdict — 104 published require entry point(s) across 67 package(s) load and 9 package(s) — all bundles in sync. check:type-check-debt OOM'd at 4 GB (also exit 3, also not a pass); re-run at 8 GB it reports 5 ledger entr(ies) re-measured, none above its recorded number.
• check:route-envelope run explicitly (it lands in the deriver's silent bucket, dispatch-gates: a whole-tree-walk gate whose workflow names: lists only its CURRENT members is placed Silent, so it is never derived for the card that adds a new member — measured on check:route-envelope / PR #16730 #16828): exit 0. This diff adds no route module — grep -rn 'c\.json\|res\.json' packages/plugins/plugin-reports/src is empty — so it was never in scope, but it was run rather than assumed. check:error-code-casing run explicitly for the same reason (its roster sits under packages/): exit 0.
• pnpm lint — the repo-wide run, not a narrowed one: eslint . --no-inline-config, exit 0.
• Tests / typecheck — @objectstack/plugin-reports 88 passed (5 files, 9 new), @objectstack/platform-objects 545 passed (37 files); both packages' typecheck green including their test layers via check:test-typecheck.
• check:test-source-alias caught a real defect in this PR's first draft and it was fixed, not routed around. The new @objectstack/spec/shared import is a value import (the type-only imports beside it are erased), so unaliased it resolved to spec's dist/ — the timezone tests would have been a verdict about build state, and a dist merely behind would have passed them against the old membership answer silently. vitest.config.ts gains the anchored array-form rule for spec's uniform namespace map, the same shape packages/metadata and packages/runtime carry. Suite re-run against source afterwards: still 88/88.

Ablation — four enforcement points, each mutated alone

Each leg: mutate → prove it reached disk (grep -c on both the removed anchor and the injected text, plus git hash-object differing from the HEAD blob) → run → restore with git checkout HEAD -- ABSPATH under trap ... EXIT INT TERM → prove restoration by an empty git diff HEAD and blob equality. HEAD blob of report-service.ts: d11eff8969637d2e5737feea308adfdec8411d74.

All four restored byte-identically (restored blob = d11eff89… = head, git diff HEAD empty); the final whole-tree git diff HEAD --stat printed nothing.

The subject of every leg is plugin-reports' own source, imported by the suite relatively (./report-service.js) and with @objectstack/spec/shared now aliased to source — so no leg's verdict is a function of any dist/, which is the condition scripts/ablation-dist-preflight.mjs guards.

Clause ② — re-derived mechanically from the delivered diff

The claim comment declares Clause-②: no. Re-derived on the final diff, not inherited:

node scripts/pm/check-widening-tells.mjs --declaration no --diff THIS_DIFF
  -> exit 0
  ✓ check-widening-tells: 5 changed file(s) read, no widening tell on any declared surface.

node scripts/pm/dispatch-gates.mjs --tier
  -> Model tier — no path-derived mandate: the surface hits none of the 3 declared glob(s).

Consistent with the reading by hand: no newly exported symbol (effectiveTimezone / isUsableTimezone are module-private), no new error code (VALIDATION_FAILED is this file's existing prefix), no new key on any stored or returned shape (last_status / last_error / failed all pre-exist), and no new last_status enum member. The direction is narrowing throughout. The declaration was not edited by this seat, and the level correction above does not depend on it — the level axis was driven both ways and passes either.

验收备注

• The accept-set change is measurably not the breaking axis (see the probe above). The breaking axis is the withdrawal of a currently-running delivery on a pre-existing row. Reviewers weighing the level should weigh that one.
• Behaviour change for stored rows: a cron schedule with a non-member zone stops delivering instead of delivering on the interval cadence. That is the intent — it was delivering at instants nobody asked for — but it is a change an operator can notice as "the digest stopped", which is why last_error says what to correct and why the changeset carries a banner.
• Noted, not filed: the sweep re-marks a quarantined row every tick because the row stays due (pre-existing shape, shared with the missing-report arm). Noted, not filed: check:route-envelope's absence from the derived union is dispatch-gates: a whole-tree-walk gate whose workflow names: lists only its CURRENT members is placed Silent, so it is never derived for the card that adds a new member — measured on check:route-envelope / PR #16730 #16828, already tracked. Corroborated, not re-filed: the level axis's nested-package blind spot is [finding] The changeset LEVEL axis is blind to every NESTED package: packages/*/src/** matches one segment, so 51 of 74 workspace packages (all drivers/services/adapters) can pair Clause-②: yes with patch and stay green #16713.

🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37